Remove < Python 3.6 compatibility code by kamyar · Pull Request #3988 · python-poetry/poetry

kamyar · 2021-04-22T22:03:53Z

Hey, I saw the issue linked below and immediately thought of pyupgrade and as poetry already uses pre-commit added the config for it with target set to python 3.6.

I know the issue says "please keep the pull requests small and modular." but as the changes are autogenerated and rather simple(in terms of mental focus needed to review) I thought might be ok to submit them together.

Please let me know if multiple small PRs is still preferred, I will open related syntax updates together (either by file/directory or by type of change)

I already went through all changes twice and did not notice anything that would change the behaviour.
Also please let me know if there is something missing.

p.s. thank you for such a great tool, it has been a true delight to use it both at work and personal project. :)

p.p.s. below are some suggested extra changes that convert f"{foo}" to foo, if they look good I will create a single commit with the fixes rather than creating one commit per suggestion.

Pull Request Check List

Resolves: #3860

Added tests for changed code. (Not Needed?)
Updated documentation for changed code. (Not Needed?)

Co-authored-by: Kamyar Ghasemlou <github@kamy.me> Co-authored-by: Harry Peach <harrypeach1999@gmail.com>

danieleades · 2021-05-10T07:35:59Z

worth also removing

poetry/poetry/repositories/installed_repository.py

Line 69 in 9591e88

# this is required to handle pathlib oddity on win32 python==3.5

in this pull request?

kamyar · 2021-05-10T08:22:54Z

@danieleades I don't have context on how to improve that but happy to help, do you have any suggestions?

danieleades · 2021-05-10T08:32:56Z

@danieleades I don't have context on how to improve that but happy to help, do you have any suggestions?

My suggestion is to remove the try/except entirely. If the comment is to be believed, it's only required for python 3.5, which isn't supported anyway

kamyar · 2021-05-10T09:19:07Z

Sounds good, will give it a try by replacing the try/except with path = lib.joinpath(path).resolve().
cc @abn fyi hope this is compatible with the scope of the original issue. :)

Co-authored-by: Kamyar Ghasemlou <github@kamy.me> Co-authored-by: Harry Peach <harrypeach1999@gmail.com>

kamyar · 2021-05-13T15:00:21Z

@abn can you take a look and allow the workflow runs? :)
I think the PR is ready, thank you

Leonardofreua · 2021-07-19T21:26:24Z


        try:
-            r = urlopen(url + "{}".format(checksum))
+            r = urlopen(url + f"{checksum}")


Couldn't this concatenation be like that?

Suggested change

r = urlopen(url + f"{checksum}")

r = urlopen(f"{url}{checksum}")

Leonardofreua · 2021-07-19T21:27:20Z


        try:
-            r = urlopen(url + "{}".format(name))
+            r = urlopen(url + f"{name}")


Here too

Suggested change

r = urlopen(url + f"{name}")

r = urlopen(f"{url}{name}")

Leonardofreua · 2021-07-19T21:35:37Z

+                self.line(f"{self.poetry.package.pretty_version}")
            else:
                self.line(
                    "<comment>{}</> <info>{}</>".format(


It's necessary to change this excerpt.

Leonardofreua · 2021-07-19T21:49:53Z

-                base_name = "poetry-{}-{}".format(__version__, sys.platform)
-                name = "{}.tar.gz".format(base_name)
+                base_name = f"poetry-{__version__}-{sys.platform}"
+                name = f"{base_name}.tar.gz"


How about unifying this string into one?

Obs.: if you accept, don't forget to change the lines that use the string name

Suggested change

name = f"{base_name}.tar.gz"

base_name = f"poetry-{__version__}-{sys.platform}.tar.gz"

Leonardofreua · 2021-07-19T22:15:03Z

-    expected = "Deleted virtualenv: {}\n".format(
-        (venv_cache / "{}-py3.6".format(venv_name))
-    )
+    expected = "Deleted virtualenv: {}\n".format(venv_cache / f"{venv_name}-py3.6")


Suggested change

expected = "Deleted virtualenv: {}\n".format(venv_cache / f"{venv_name}-py3.6")

expected = f"Deleted virtualenv: {venv_cache / venv_name}-py3.6\n"

danieleades · 2021-09-16T12:00:42Z

is this pull request still alive?

HarryPeach · 2021-10-14T21:10:56Z

is this pull request still alive?

I hope so, would be nice to get the code merged as it seems ready.

@abn Is there anything that needs changing on this before the workflows can be run? Would be happy to contribute some more to get this over the line

Implements the reviews from #3987 and #3988. Special thanks to the committers and reviewers there. Co-authored-by: Harry Peach <harrypeach1999@gmail.com> Co-authored-by: Kamyar Ghasemlou <kamyar@kamy.me> Co-authored-by: Leonardo Freua <leonardo.batista.freua@gmail.com>

neersighted · 2021-11-15T02:43:52Z

I'm closing this in favor of #4758 as it uses the current pre-commit structure, skips files that should not be altered by pyupgrade (get-poetry.py and install-poetry.py), and because there have been enough changes to master that a new run is needed.

Thank you for the work on these PRs! I've incorporated the changes from review into my version, and given credit where due. Please submit more PRs cleaning out old code/cruft when you can!

Implements the reviews from #3987 and #3988. Special thanks to the committers and reviewers there. Co-authored-by: Harry Peach <harrypeach1999@gmail.com> Co-authored-by: Kamyar Ghasemlou <kamyar@kamy.me> Co-authored-by: Leonardo Freua <leonardo.batista.freua@gmail.com>

github-actions · 2024-02-29T20:08:54Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

kamyar force-pushed the master branch from 30ab52d to 3409bce Compare April 22, 2021 22:14